Skip to content

Migrated to PyConcrete* base classes#9745

Open
jpienaar wants to merge 4 commits intollvm:mainfrom
jpienaar:pyconc
Open

Migrated to PyConcrete* base classes#9745
jpienaar wants to merge 4 commits intollvm:mainfrom
jpienaar:pyconc

Conversation

@jpienaar
Copy link
Member

@jpienaar jpienaar commented Feb 24, 2026

The mlir_*_subclass APIs are soft-deprecated and will be removed in the future.

Exposed more C API methods for the isa methods in HW.

Updated hw test to [IntegerType(i2), IntegerType(i32), InOutType(!hw.inout<i32>)] (Type -> InOutType - I am not sure why this change, but figured it is accurate still, but can dig in if not).

Attempted to do as mechanically as possible. But still is going to be a bit of a pain to review.

@jpienaar jpienaar requested a review from teqdruid February 26, 2026 16:31
@jpienaar jpienaar marked this pull request as ready for review February 26, 2026 16:32
Copy link
Contributor

@teqdruid teqdruid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've kicked off a pycde test run. It more completely tests the ESI bindings. I should really have more complete tests in our integration tests.

https://github.com/llvm/circt/actions/runs/22458379705

@teqdruid
Copy link
Contributor

The pycde tests failed. It seems that something has changed regarding python type conversions...

@jpienaar
Copy link
Member Author

Was able to repro. This is funny as the changes to support.py (which is used by pycde too) is exactly towards this. The checking should be on the underlying TypeID which I'd not expect to change in PyCDE. I'll instrument the isa methods to check.

@jpienaar
Copy link
Member Author

I think I see it: seq and esi both don't have the TypeID getters, that's missing.

@jpienaar jpienaar requested a review from teqdruid February 27, 2026 09:27
@teqdruid
Copy link
Contributor

teqdruid commented Mar 3, 2026

Sorry about the delay. I pulled and pushed and kicked a build off: https://github.com/llvm/circt/actions/runs/22640967493

@jpienaar
Copy link
Member Author

jpienaar commented Mar 4, 2026

Thanks, looks green in that one (unrelated, I could get cde working locally with the exclusion of the simulation, some interplay with what gRPC needed cmake side with what I have, so thanks for kicking it off as I couldn't check).

@jpienaar
Copy link
Member Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants